Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

AzureCliCredential to treat datetime without TZ info as local time. #5179

Merged
merged 12 commits into from
Dec 16, 2023
Merged

AzureCliCredential to treat datetime without TZ info as local time. #5179

merged 12 commits into from
Dec 16, 2023

Conversation

antkmsft
Copy link
Member

@antkmsft antkmsft commented Nov 23, 2023 •
edited
Loading

Closes #5075.

This is not a replacement for #5180, because that one will work only if the user has the new version of Azure CLI installed.

Obsoletes #5076.

@antkmsft antkmsft mentioned this pull request Nov 23, 2023
Closed
14 tasks
@antkmsft antkmsft marked this pull request as ready for review November 23, 2023 05:56
@antkmsft antkmsft mentioned this pull request Nov 23, 2023
Merged
@antkmsft antkmsft self-assigned this Nov 27, 2023
@antkmsft antkmsft requested a review from RickWinter November 28, 2023 08:47
@antkmsft antkmsft requested a review from ahsonkhan November 30, 2023 21:11
ahsonkhan
ahsonkhan reviewed Dec 14, 2023
View reviewed changes
Copy link
Member

@ahsonkhan ahsonkhan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Otherwise, lgtm.

Holding off approval until after #5180 is merged.

@antkmsft antkmsft requested a review from ahsonkhan December 15, 2023 04:51
ahsonkhan
ahsonkhan approved these changes Dec 16, 2023
View reviewed changes
Copy link
Member

@ahsonkhan ahsonkhan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a much cleaner and isolated fix (without requiring extra parsing logic in DateTime within Azure::Core). Appreciate it!

@antkmsft antkmsft merged commit a419a63 into Azure:main Dec 16, 2023
78 checks passed
@antkmsft antkmsft deleted the azcli-localtime branch December 16, 2023 01:40
This was referenced Jan 8, 2024
Closed
Closed
Closed
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ahsonkhan ahsonkhan ahsonkhan approved these changes

@schaabs schaabs Awaiting requested review from schaabs schaabs is a code owner

@vhvb1989 vhvb1989 Awaiting requested review from vhvb1989

@gearama gearama Awaiting requested review from gearama gearama is a code owner

@LarryOsterman LarryOsterman Awaiting requested review from LarryOsterman LarryOsterman is a code owner

@pvaneck pvaneck Awaiting requested review from pvaneck pvaneck is a code owner automatically assigned from Azure/azure-sdk-write-identity

@KarishmaGhiya KarishmaGhiya Awaiting requested review from KarishmaGhiya KarishmaGhiya is a code owner automatically assigned from Azure/azure-sdk-write-identity

@chlowell chlowell Awaiting requested review from chlowell chlowell is a code owner automatically assigned from Azure/azure-sdk-write-identity

@mpodwysocki mpodwysocki Awaiting requested review from mpodwysocki mpodwysocki is a code owner automatically assigned from Azure/azure-sdk-write-identity

@xiangyan99 xiangyan99 Awaiting requested review from xiangyan99 xiangyan99 is a code owner automatically assigned from Azure/azure-sdk-write-identity

@RickWinter RickWinter Awaiting requested review from RickWinter RickWinter is a code owner

Assignees

@antkmsft antkmsft

Labels
Azure.Identity
Projects
Azure Identity SDK Improvements
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

az cli returns expiresOn in local time but sdk assumes c++ time
3 participants
@antkmsft @RickWinter @ahsonkhan